Skip to content

feat(ui): Settings screen matching Figma 17:2 (#64)#71

Merged
ilmoniemi merged 3 commits into
mainfrom
feature/64
May 13, 2026
Merged

feat(ui): Settings screen matching Figma 17:2 (#64)#71
ilmoniemi merged 3 commits into
mainfrom
feature/64

Conversation

@ilmoniemi
Copy link
Copy Markdown
Contributor

@ilmoniemi ilmoniemi commented May 13, 2026

Summary

  • Replaces SettingsPlaceholder with SettingsScreen at app/src/main/java/de/pyryco/mobile/ui/settings/SettingsScreen.kt, mirroring Figma node 17:2 — TopAppBar + back nav, vertically scrollable Column over seven label-large section headers (Connection, Appearance, Defaults for new conversations, Notifications, Memory, Storage, About) with M3 ListItem rows.
  • Row trailing affordances: 20dp chevron (KeyboardArrowRight), 18dp external-link icon (local vector drawable), M3 Switch, + Add pill (TextButton), or none — matching the AC table exactly. Three local-only mutableStateOf switches (Material You = on, YOLO = off, Push = on) animate during manual QA; no ViewModel, no persistence.
  • Wires composable(Routes.Settings) in MainActivity.kt to the new screen and removes the orphaned SettingsPlaceholder, Column, and TextButton imports.

Implementation notes

  • No new dependency. The architect's spec referenced Icons.Default.OpenInNew, but that icon lives in material-icons-extended, which isn't on the classpath (only material-icons-core). Per the "no new dependencies without justification" rule I added a small local vector drawable at res/drawable/ic_open_in_new.xml instead of pulling in the ~5 MB extended icon library for a single glyph.
  • "Add" button = TextButton, not OutlinedButton. The Figma 17:81 frame has rounded-100 geometry but the rendered screenshot shows no visible border. The architect's spec called this out as developer's choice; TextButton matches the visual reference cleanly.
  • Two @Preview composables (light + dark) at widthDp = 412 matching the rest of the codebase.

Issue

Closes #64.

Testing

  • ./gradlew assembleDebug — clean
  • ./gradlew test — clean (no new unit tests; the screen has no business logic per spec)
  • ./gradlew lint — 0 errors; 29 warnings all pre-existing (gradle versions, theme ObsoleteSdkInt, etc.), none introduced by this change
  • ./gradlew connectedAndroidTest — not run (no device connected in this environment); no instrumented tests added per spec

Architecture compliance

Follows the spec at docs/specs/architecture/64-settings-screen-figma-17-2.md verbatim except for the two implementation notes above (OpenInNew drawable substitution; TextButton for Add pill — explicit alternative permitted by the spec's open question). Public surface is the single SettingsScreen(onBack, modifier) symbol; all helpers (SettingsSectionHeader, SettingsRow, ChevronIcon, ExternalLinkIcon, AddPill) are private. No SettingsViewModel, no Koin module, no repository touched.

🤖 Generated with Claude Code

ilmoniemi added 2 commits May 13, 2026 22:15
@ilmoniemi
spec: Settings screen matching Figma 17:2 (#64)
a464617
@ilmoniemi
feat(ui): Settings screen matching Figma 17:2 (#64)
cde4faf 
Replace the SettingsPlaceholder stub with a sectioned Settings
screen mirroring Figma node 17:2: M3 TopAppBar + back nav, seven
labelled sections, M3 ListItem rows with chevron / Switch / Add /
external-link / no trailing affordances. Pure stateless composable
plus three local-only switches; no ViewModel, no persistence.

OpenInNew icon is provided as a local vector drawable rather than
pulling in material-icons-extended.
@ilmoniemi
Copy link
Copy Markdown
Contributor Author

ilmoniemi commented May 13, 2026

Code Review: #64

Decision: PASS

Findings

  • [NIT] SettingsScreen.kt:63-65 — the three switch states are declared inside the Scaffold { inner -> } content lambda. Functionally fine (remember is keyed to its call site), but hoisting them above the Scaffold block would read more conventionally as "screen-scoped UI state" and matches the pattern used elsewhere in the codebase. Pure style; do not block.
  • [NIT] SettingsScreen.kt:213 — ListItemDefaults.colors(containerColor = MaterialTheme.colorScheme.surface) is redundant; surface is already the default container for ListItem. Removing the explicit override would shrink the file by 5 LOC without changing behaviour. Leave or remove at the developer's discretion.

Summary

Faithful, minimal implementation of the spec. Visual fidelity against Figma 17:2 confirmed: section order, section-header color (Schemes/primary), row affordances (chevron / Switch / "+ Add" pill / OpenInNew / none), and switch defaults (Material You = on, YOLO = off, Push = on) all match the rendered design. Zero hardcoded color/typography literals; all user-facing strings (title, content descriptions) routed through strings.xml; subtitle placeholders inline at call sites per the spec's explicit guidance. Accessibility correctly handled — decorative chevron/external-link icons use contentDescription = null; the interactive Add icon has a real content description.

The two judgement calls flagged in the PR body are both reasonable:

  1. Local vector drawable for OpenInNew instead of pulling material-icons-extended for a single glyph — correct trade-off. The drawable's hardcoded fillColor=@android:color/white is harmless because Compose's Icon overrides tint with LocalContentColor regardless.
  2. TextButton for the "Add" pill instead of OutlinedButton — matches the borderless rendering in the Figma screenshot; the spec's open question explicitly permitted this.

MainActivity diff is exactly what the spec called for (~3 lines + import + orphan-import cleanup). No SettingsViewModel, no Koin module, no data/ touched — visual stub only, as the ticket scoped.

Approving for documentation.

@ilmoniemi @claude
docs: Settings screen — feature doc, per-ticket notes, INDEX entry (#64)
7434e26 
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ilmoniemi ilmoniemi merged commit d9c2c44 into main May 13, 2026
@ilmoniemi ilmoniemi deleted the feature/64 branch May 13, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(ui): replace Settings stub with screen matching Figma node 17:2

1 participant

@ilmoniemi